Skip to content

[AMD] [AGENTX] GLM-5.2: EP=1 interactivity + c12 throughput tuning on MI355X - #2777

Merged
Oseltamivir merged 10 commits into
mainfrom
pr/glm52-sglang-ep1-c12
Sep 1, 2026
Merged

[AMD] [AGENTX] GLM-5.2: EP=1 interactivity + c12 throughput tuning on MI355X#2777
Oseltamivir merged 10 commits into
mainfrom
pr/glm52-sglang-ep1-c12

Conversation

@giovanniguastiamd

@giovanniguastiamd giovanniguastiamd commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary / 摘要

Tuning follow-up to #2570 (GLM-5.2 FP4 MI355X SGLang Agentic MTP).

Changes / 变更内容

1. TP8 arm: EP=8 → EP=1 (script + yaml)

GLM-5.2 is a MoE model. With EP=8 every forward pass introduces an all-to-all collective between 8 EP ranks to route tokens to the correct experts. At low concurrency (c4, effective batch ≈ 1–2 requests) this collective is overhead on essentially unshared traffic.

Switching to EP=1 (no expert parallelism, all experts replicated across TP=8 ranks) eliminates the all-to-all entirely:

  • ITL p50 at c4: 7.3 ms → 6.95 ms (−5%)
  • Interactivity P90 at c4: 105 → 110.5 tok/s/user (+5%)

The TP8 concurrency sweep is extended to [4, 6, 8, 10] to cover the mid-range crossover region (previously only [1, 2, 4]).

2. HiCache tuning: ratio 1.5 → 2.5, write_through_selective (script)

The agentic-coding corpus (ISL p50 ≈ 90k tokens) saturates any fixed DRAM KV pool at conc ≥ 10. Raising the host-DRAM ratio to 2.5× delays saturation and keeps throughput stable through conc 12:

  • tok/s/GPU at c12: 102 vs 91 at c10 (+12%)

write_through_selective (introduced in #2679) skips DRAM writes for non-reusable KV blocks, reducing host-bus traffic without impacting cache hit rate.

The TP4 concurrency list is trimmed to [1, 2, 4, 8, 10, 12]; conc 16 is removed (untested and beyond the validated range).

3. MAX_RUNNING_REQUESTS 1×CONC → 2×CONC; CUDA_GRAPH_MAX_BS capped at 64 (script)

With MTP num-steps=5 the draft+verify phase transiently batches more tokens than CONC sessions. Doubling the in-flight slot budget prevents scheduler stalls under burst. SGLang auto-interpolates a batch-size list [1..max_bs], so capping cuda-graph-max-bs at 64 keeps graph-capture memory bounded without losing coverage.


摘要

#2570(GLM-5.2 FP4 MI355X SGLang Agentic MTP)的调优跟进。

1. TP8 臂:EP=8 → EP=1:GLM-5.2 是 MoE 模型,EP=8 在每次前向计算时引入 all-to-all 集合通信。低并发(c4)下该通信几乎无法分摊,EP=1 消除了这一开销:c4 ITL p50 7.3→6.95 ms(−5%),P90 交互性 105→110.5 tok/s/user(+5%)。TP8 并发扫描扩展至 [4, 6, 8, 10]。

2. HiCache 调优:ratio 1.5→2.5,write policy 改为 write_through_selective。agentic-coding 语料在 conc ≥ 10 时会填满任何固定 DRAM 池;更大的 host 端为 c12 提供吞吐支撑(+12% tok/s/GPU)。TP4 并发列表缩减为 [1, 2, 4, 8, 10, 12]。

3. MAX_RUNNING_REQUESTS 1×→2×CONC:MTP draft+verify 阶段瞬时 batch 超过 CONC,加大 in-flight slot 可防止调度器阻塞。CUDA_GRAPH_MAX_BS 上限设为 64(SGLang 已自动插值 bs 列表)。


Test plan / 测试计划

  • Full sweep on MI355X cluster: TP4+HiCache conc [1,2,4,8,10,12] and TP8+EP=1 conc [4,6,8,10] both produce valid results
  • Interactivity P90 at c4 ≥ 108 tok/s/user (within 2% of measured 110.5)
  • Throughput at c12 ≥ 98 tok/s/GPU (within 4% of measured 102)
  • No OOM on TP8/EP=1 (expert weights replicated across 8 ranks; HBM budget verified pre-run)

Note

Medium Risk
Changes MoE parallelism, host DRAM pinning policy, and in-flight scheduling limits for production benchmark sweeps; mis-sized HiCache or TP8 HBM can cause OOM or skewed perf curves on cluster runners.

Overview
Tuning for glm5.2-fp4-mi355x-sglang-agentic-mtp on MI355X: benchmark script defaults and amd-master.yaml search space, plus a perf-changelog.yaml entry.

The TP8 low-latency arm switches from EP=8 to EP=1 (no MoE all-to-all) with GPU-resident KV; concurrency is [1, 2, 4, 10] after sweep validation dropped mid-range points that lose to the TP4/HiCache arm. The TP4/EP4 HiCache arm keeps DRAM offload but trims concurrency to [1, 2, 4, 8, 10, 12] (removes untested 16).

In glm5.2_fp4_mi355x_sglang_mtp.sh, HiCache defaults stay HICACHE_RATIO=1.5 for ~3 TB cluster nodes with docs/env override for 2.5 on larger DRAM; write_through_selective replaces write_through. Scheduler headroom raises MAX_RUNNING_REQUESTS to 2×CONC for MTP burst batching and caps CUDA_GRAPH_MAX_BS at 64 to bound graph-capture memory.

Reviewed by Cursor Bugbot for commit 82bb99b. Bugbot is set up for automated code reviews on this repo. Configure here.

giovanniguastiamd and others added 2 commits August 28, 2026 10:21
…e tuning

- Switch TP8 arm from EP=8 to EP=1: eliminates MoE all-to-all, ITL p50
  -5% at c4 (6.95→7.3 ms), P90 interactivity +5% (110.5 vs 105 tok/s/user).
  Extend TP8 sweep to conc [4, 6, 8, 10].
- Raise HiCache ratio 1.5→2.5, write policy write_through→write_through_selective:
  sustains throughput through c12 (+12% tok/s/GPU vs c10 baseline).
  Trim TP4 sweep to [1, 2, 4, 8, 10, 12].
- Double MAX_RUNNING_REQUESTS to 2×CONC for MTP draft+verify headroom;
  cap CUDA_GRAPH_MAX_BS at 64 (SGLang auto-interpolates the bs list).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

2 similar comments
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@seungrokj seungrokj added AMD full-sweep-fail-fast agentx AgentX benchmarks, recipes, and infrastructure labels Aug 28, 2026
Comment on lines +73 to +89
# Per-arm L2 ratio (sizing rationale below) applies to both backends unless
# overridden via HICACHE_RATIO. TP arm (182.7 GB/rank device pool): the
# working set oversubscribes the device pool ~3x at conc 32, so the host
# tier is what carries the radix hits - ratio 1.5 (~2.9 TB pinned incl.
# sidecars) validates through the conc-24 long-context storm for the
# mooncake arm. The DP-attention arm (159.4 GB/rank) only runs at conc >=
# 32, where each DP rank's ~8 sessions nearly fit in its own device pool
# (~1.5-1.6M of 1.7M tokens at conc 64) and the host tier just absorbs
# overflow - ratio 1.5 boots but the host OOM killer takes the server
# mid-storm at conc 48, so it runs ratio 0.5 (~1.2 TB pinned, ~1.8 TB of
# load headroom) at negligible hit-rate cost. The hicache-only arm has no
# L3 to fall back on, so these ratios are unvalidated there - override with
# HICACHE_RATIO if the host OOMs or hit-rate is poor.
# agentic-coding corpus saturates any fixed DRAM pool at conc ≥ 10; ratio 2.5
# (~4.8 TB pinned) sustains throughput through conc 12 and leaves headroom for
# the mooncake arm's conc-24 long-context storm. The DP-attention arm
# (159.4 GB/rank) only runs at conc >= 32, where the host tier just absorbs
# overflow - ratio 0.5 (~1.2 TB pinned, ~1.8 TB of load headroom) at negligible
# hit-rate cost (ratio 1.5 OOMs the host mid-storm at conc 48).
CACHE_ARGS=()
if agentic_kv_offload_enabled; then
if [ "$DP_ATTENTION" = "true" ]; then
HICACHE_RATIO="${HICACHE_RATIO:-0.5}"
else
HICACHE_RATIO="${HICACHE_RATIO:-1.5}"
# ratio=2.5 (vs the former 1.5): the agentic-coding corpus fills any
# fixed DRAM pool at conc ≥ 10; a larger host tier delays saturation
# and keeps throughput stable through conc 12.
HICACHE_RATIO="${HICACHE_RATIO:-2.5}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 HICACHE_RATIO default raised from 1.5 to 2.5 for the TP4/EP4 arm, and the script's own sizing comment puts the resulting pinned host memory at ~4.8 TB. configs/runners.yaml (lines 281-283) lists cluster:mi355x-amds available-cpu-dram-mib as 3,095,781 (~3.0-3.25 TB total node DRAM), so the new default requests more pinned host memory than physically exists on the node, before even accounting for the 0.8 dram-utilization headroom or other processes. HICACHE_RATIO is a bare env-var default (line 89) with no runtime check against TOTAL_CPU_DRAM_GB (validated only for positivity in benchmark_lib.sh, never used to size the ratio) or against the node's hardware capacity in runners.yaml, so nothing stops this mismatch from reaching the server launch.

Extended reasoning...

Running the TP4/EP4 arm at conc>=8-12 (its own comment says the corpus saturates the pool at conc>=10) makes HiCache try to pin close to the ~4.8 TB implied by ratio=2.5, exceeding the node's ~3-3.25 TB of DRAM; the host OOM killer kills the sglang server mid-run, the same failure mode the file's own comments describe happening to the DP-attention arm at ratio 1.5. At the prior ratio (1.5, ~2.9 TB per the old comment) the request fit under the node's real capacity; the new default does not. A correct fix would derive/clamp the pinned-memory target from TOTAL_CPU_DRAM_GB (already computed per-node from dram-utilization and GPU-count fraction) rather than hardcoding a ratio disconnected from actual host capacity.

Verification: normal. benchmarks/single_node/agentic/glm5.2_fp4_mi355x_sglang_mtp.sh:89 raises the TP4/EP4 arm default HICACHE_RATIO from 1.5 to 2.5, passed unclamped to --hicache-ratio (line 101). The merged code's own sizing comment states ratio 2.5 pins ~4.8 TB of host memory (lines 75-76: "ratio 2.5 (~4.8 TB pinned)"). configs/runners.yaml:281-282 lists cluster:mi355x-amds available-cpu-dram-mib:…

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giovanniguastiamd plz check HICACHE_RATIO

@github-actions

Copy link
Copy Markdown
Contributor

Sweep validation shows TP8/EP=1 at c6 (P90=91.3) and c8 (P90=72.7)
are dominated by the TP4/EP4/HiCache arm at those concurrency points.
Keeping only c4 (best interactivity anchor) and c10 (crossover bound).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@giovanniguastiamd giovanniguastiamd changed the title [AMD] [AGENTX] [WIP] GLM-5.2: EP=1 interactivity + c12 throughput tuning on MI355X [AMD] [AGENTX] GLM-5.2: EP=1 interactivity + c12 throughput tuning on MI355X Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

…ew bullet, fix pr-link 2769→2777

The append-only validator rejects modifications to existing description bullets.
Restore the first bullet to its original wording (with [4,6,8,10] sweep text) and
append a new fourth bullet describing the c6/c8 trim. Also update pr-link to 2777.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@seungrokj

Copy link
Copy Markdown
Collaborator

seungrokj and others added 2 commits August 31, 2026 15:45
… as env-var override

ratio=2.5 (~4.8 TB pinned) exceeds the ~3.0 TB available DRAM on
cluster:mi355x-amds nodes (runners.yaml available-cpu-dram-mib: 3_095_781).
HiCache allocates lazily so runs did not OOM, but the declared default was
unsafe. Revert to 1.5 (~2.9 TB, safe within node DRAM budget) and document
that 2.5 can be set via HICACHE_RATIO env-var on nodes with >4 TB DRAM.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 21349fa. Configure here.

Comment thread benchmarks/single_node/agentic/glm5.2_fp4_mi355x_sglang_mtp.sh
@github-actions

Copy link
Copy Markdown
Contributor

@seungrokj

Copy link
Copy Markdown
Collaborator

/reuse-sweep-run

@seungrokj seungrokj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a PR reviewer and CODEOWNER, I have reviewed this and have:

  • Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
  • Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
  • Verified that this PR has passed PR validation. Please link to GitHub Action workflow that shows this.
  • Verified that this PR passes evals. Please link to GitHub Action workflow that shows this.
  • Verified that speculative decoding PRs uses chat templates to align the AL distribution to real world
  • For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.
  • Verified against the current MODELS.md that this PR does not submit a deprecated model, scenario, or model-scenario combination.
  • Verified that the model architecture isn't changed with benchmark hacks like using --hf-overrides to skipping indexer for every x layers on models that don't natively support this. As a general rule, we won't accept optimizations that reduces the number of model architecture FLOPs. Anything that makes that same computation run faster is fair game; FLOPs at lower precisions is fine, given that the config passes private evals. As an general north star princple, we should only use optimizations which is used in production by customers that care about accuracy
  • If an company claims that they support vLLM/SGLang as first class LLM inference engines on their hardware, I have verified that the respective vLLM submission made using upstream https://hub.docker.com/u/vllm docker repo, upstream SGLang https://hub.docker.com/u/lmsysorg docker repo. The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet as supported by vLLM/SGLang community maintainers
  • If an company claims that they support vLLM/SGLang as first class upstream in-tree LLM inference engines on their hardware, I have have verified that the respective vLLM/SGLang submission has been made before additional frameworks (TRT-LLM, ATOM, etc.). The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet.
  • Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or the SGLang cookbook:
    • I linked the corresponding upstream PR in the vLLM recipe repo or SGLang repo and verified that it is MERGED before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below.
  • Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at docs/waiver/<PR_NUMBER>.md — named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section.
  • If this PR uses append-only: true, verified that it only adds generated points or recipe variants inside a selected existing config/scenario and existing same-image visual curve: every previously generated point remains present with the same recipe, no prior point is removed or rerun, and every benchmark-affecting change in the complete diff can affect only the corresponding newly appended points (never an existing point), regardless of which file contains it.
  • If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.

Additional detail section:

Signed: seungrokj

@Klaud-Cold

Klaud-Cold commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

❌❌❌ REJECTED ❌❌❌

@seungrokj — one blocker remains (the previously flagged unchecked checklist items are now fixed in the updated sign-off): the published SGLang cookbook recipe for GLM-5.2 MI355X MXFP4 does not cover this PR's major server args — its MXFP4 cells are TP4 with no --ep-size, and its only MTP variant is 3-1-4, explicitly stating --speculative-num-steps > 3 hits a gfx950 build issue — while this PR serves MTP 5-1-6 on TP4/EP4 and TP8/EP1 arms.

✅ Check 0 (CODEOWNER): PASS — @seungrokj is a listed owner of configs/amd-master.yaml; the other changed paths are catch-all-only.
✅ Check 1 (sweep on in-PR commit): PASS — in-PR commit 3638c3e has executed green agentic / (all 10 matrix points) and agentic eval / check-runs in run 33373633743.
✅ Check 2 (eval accuracy): PASS — GSM8K em_strict 0.9666 (n_eff 1319) ≥ 0.90 bar (utils/evals/thresholds.yaml default; no glm5.2 override), run on this PR's image lmsysorg/sglang-rocm:v0.5.16-rocm720-mi35x-20260728.
❌ Check 3 (recipe): FAIL — major args missing from / contradicting the linked published recipe (GLM-5.2 cookbook page, source merged on sgl-project/sglang main at docs/cookbook/autoregressive/GLM/GLM-5.2.mdx): the PR serves --speculative-num-steps 5 --speculative-num-draft-tokens 6, but the recipe's only MI355X-MXFP4 MTP variant is 3-1-4 and states steps > 3 hits a build issue on gfx950; the recipe's MXFP4 cells are TP4 with no --ep-size (EP=1), while the PR runs TP4/EP4 plus an undocumented TP8/EP1 arm. (HiCache offload, chunked-prefill, max-running-requests, cuda-graph caps, and SGLANG_SIMULATE_ACC_* are InferenceX-specific tuning — informational only, not blockers.)
✅ Check 4 (reuse command): PASS — /reuse-sweep-run posted by @seungrokj (COLLABORATOR).
✅ Check 5 (latest checklist): PASS — the updated sign-off now checks every item of the current template, including the previously unchecked engine-patch and additional-reasoning items.
✅ Check 6 (upstream image / engine-first): PASS — lmsysorg/sglang-rocm:v0.5.16-rocm720-mi35x-20260728 is the upstream lmsysorg org, and the entry is itself SGLang.
✅ Check 7 (deprecations): PASS — GLM-5.2 agentic-coding MTP is the retained arm per MODELS.md as of 2026-08-31.
✅ Check 8 (architecture hacks): PASS — no --hf-overrides or FLOP-reducing overrides in the diff or launch command.
✅ Check 9 (spec-decode chat template): PASS — the agentic replay drives /v1/chat/completions with --endpoint-type chat (benchmarks/benchmark_lib.sh).
✅ Check 10 (engine patches): PASS — no engine patching; client/harness deps only.
✅ Check 11 (golden AL): PASS — SGLANG_SIMULATE_ACC_LEN=3.61 with match-expected/real-draft-token equals the golden AL for glm5.2 thinking_on at 5 speculative tokens (golden_al_distribution/glm5.2_mtp.yaml), matching --speculative-num-steps 5.
➖ Check 12 (append-only): N/A — no new append-only: true changelog entries.

@seungrokj
seungrokj self-requested a review August 31, 2026 16:15

@seungrokj seungrokj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a PR reviewer and CODEOWNER, I have reviewed this and have:

  • Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
  • Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
  • Verified that this PR has passed PR validation. Please link to GitHub Action workflow that shows this.
  • Verified that this PR passes evals. Please link to GitHub Action workflow that shows this.
  • Verified that speculative decoding PRs uses chat templates to align the AL distribution to real world
  • For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.
  • Verified against the current MODELS.md that this PR does not submit a deprecated model, scenario, or model-scenario combination.
  • Verified that the model architecture isn't changed with benchmark hacks like using --hf-overrides to skipping indexer for every x layers on models that don't natively support this. As a general rule, we won't accept optimizations that reduces the number of model architecture FLOPs. Anything that makes that same computation run faster is fair game; FLOPs at lower precisions is fine, given that the config passes private evals. As an general north star princple, we should only use optimizations which is used in production by customers that care about accuracy
  • If an company claims that they support vLLM/SGLang as first class LLM inference engines on their hardware, I have verified that the respective vLLM submission made using upstream https://hub.docker.com/u/vllm docker repo, upstream SGLang https://hub.docker.com/u/lmsysorg docker repo. The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet as supported by vLLM/SGLang community maintainers
  • If an company claims that they support vLLM/SGLang as first class upstream in-tree LLM inference engines on their hardware, I have have verified that the respective vLLM/SGLang submission has been made before additional frameworks (TRT-LLM, ATOM, etc.). The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet.
  • Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or the SGLang cookbook:
    • I linked the corresponding upstream PR in the vLLM recipe repo or SGLang repo and verified that it is MERGED before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below.
  • Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at docs/waiver/<PR_NUMBER>.md — named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section.
  • If this PR uses append-only: true, verified that it only adds generated points or recipe variants inside a selected existing config/scenario and existing same-image visual curve: every previously generated point remains present with the same recipe, no prior point is removed or rerun, and every benchmark-affecting change in the complete diff can affect only the corresponding newly appended points (never an existing point), regardless of which file contains it.
  • If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.

Additional detail section:

Signed: seungrokj

@seungrokj
seungrokj self-requested a review August 31, 2026 16:24

@seungrokj seungrokj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a PR reviewer and CODEOWNER, I have reviewed this and have:

  • Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
  • Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
  • Verified that this PR has passed PR validation. Please link to GitHub Action workflow that shows this.
  • Verified that this PR passes evals. Please link to GitHub Action workflow that shows this.
  • Verified that speculative decoding PRs uses chat templates to align the AL distribution to real world
  • For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.
  • Verified against the current MODELS.md that this PR does not submit a deprecated model, scenario, or model-scenario combination.
  • Verified that the model architecture isn't changed with benchmark hacks like using --hf-overrides to skipping indexer for every x layers on models that don't natively support this. As a general rule, we won't accept optimizations that reduces the number of model architecture FLOPs. Anything that makes that same computation run faster is fair game; FLOPs at lower precisions is fine, given that the config passes private evals. As an general north star princple, we should only use optimizations which is used in production by customers that care about accuracy
  • If an company claims that they support vLLM/SGLang as first class LLM inference engines on their hardware, I have verified that the respective vLLM submission made using upstream https://hub.docker.com/u/vllm docker repo, upstream SGLang https://hub.docker.com/u/lmsysorg docker repo. The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet as supported by vLLM/SGLang community maintainers
  • If an company claims that they support vLLM/SGLang as first class upstream in-tree LLM inference engines on their hardware, I have have verified that the respective vLLM/SGLang submission has been made before additional frameworks (TRT-LLM, ATOM, etc.). The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet.
  • Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or the SGLang cookbook:
    • I linked the corresponding upstream PR in the vLLM recipe repo or SGLang repo and verified that it is MERGED before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below.
  • Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at docs/waiver/<PR_NUMBER>.md — named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section.
  • If this PR uses append-only: true, verified that it only adds generated points or recipe variants inside a selected existing config/scenario and existing same-image visual curve: every previously generated point remains present with the same recipe, no prior point is removed or rerun, and every benchmark-affecting change in the complete diff can affect only the corresponding newly appended points (never an existing point), regardless of which file contains it.
  • If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.

Additional detail section:

Signed: seungrokj

@seungrokj

seungrokj commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

It seems Klaud-Cold cannot capture the EAGLE setting in the sgl cookbook

image

@Oseltamivir Oseltamivir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Oseltamivir
Oseltamivir merged commit 3a7d6b7 into main Sep 1, 2026
31 checks passed
@Oseltamivir
Oseltamivir deleted the pr/glm52-sglang-ep1-c12 branch September 1, 2026 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentx AgentX benchmarks, recipes, and infrastructure AMD full-sweep-fail-fast

Projects

Development

Successfully merging this pull request may close these issues.

4 participants